Repository navigation
LINODE: BUGFIX: too picky about hyphens in names of SRV records - #4828
Conversation
|
CC @dairiki: Please test. There's a bugfix release going out in an hour or so. I'd love to include this, but I understand that's a tight schedule! Thanks! |
dairiki
left a comment
dairiki
left a comment
There was a problem hiding this comment.
Thank you for working on this!
I've tested this. It works for hyphenated service names.
As it stands, this does not work for SRV records on subdomains. See in-line comments for suggestions on a fixed and improved regexp.
Co-authored-by: Jeff Dairiki <dairiki@dairiki.org>
|
PTAL |
|
@TomOnTime Otherwise, it looks good to me. Thank you! |
|
Never let a regex do what an algorithm can do better. I've rewritten this to use strings.SplitN() instead of regex extraction. That allowed a much more simple regular expression. It is less strict that the API, but that's ok. It's the API's job to validate. We can only do so much. PTAL |
|
When you test it, please make sure none of the integration tests are automatically skipped. |
dairiki
left a comment
dairiki
left a comment
There was a problem hiding this comment.
SRV priority of zero is allowed.
The latest changes have re-broken SRV records on subdomains.
I'm working on running the integration tests. Some failures were reported the first time I ran them. I ran them again, with the output captured for better examination, and ... no errors. (My familiarity with go is zero, so it will take awhile to suss this out.)
There appear to be no integration tests that test the creation of SRV records on subdomains.
|
|
||
| Linode requires [`SRV`](../language-reference/domain-modifiers/SRV.md) records to have a non-zero priority. | ||
| Linode requires [`SRV`](../language-reference/domain-modifiers/SRV.md) records | ||
| to have a non-zero priority. Linode validates service name and protocol more |
There was a problem hiding this comment.
A SRV priority of zero works with the Linode API. (I've tested it.) The priority, for both SRV and MX records, does seem to be limited to the range [0-255], however.
| } | ||
|
|
||
| func validateSrvLabelHelper(label string) (string, string, error) { | ||
| parts := strings.SplitN(label, ".", 3) |
There was a problem hiding this comment.
This latest change has re-broken the ability to put SRV records in subdomains.
The only way to pass a subdomain to the Linode API is to include it in the "protocol" parameter. (I have no idea whether this is intentional or not, but there appears to be no other way to do it. As you have noted, it's a pretty crummy API design.)
I.e. if our zone is "example.org", and we want to create a SRV record at "_srv._proto.sub-domain.example.org", we need to pass service="srv" and protocol="proto.sub-domain" to Linode's API.
Here, the subdomain is in parts[2], which is currently being discarded.
|
All seems to work now. I can create SRV records on subdomains. I've run the integration tests again, and they appear to have passed. Thank you! |
|
Glad to help! |
2 participants
Fixes #4812